Fix introduction of decimal errors due to double decimal precision using BROTLI #611
Conversation
|
Thanks for the PR! Can you check the comments? |
|
Could you please provide a link to the comments? I am struggling to find them. |
|
Here at the bottom: https://github.com/potree/PotreeConverter/pull/611/files |
…nts outside bounding box
| min.x = 0; | ||
| min.y = 0; | ||
| min.z = 0; |
There was a problem hiding this comment.
Hi, can you tell me the reason for initializing min values with 0? As far as I can tell that would make the bounding box minimum 0, even if it is actually a large integer value.
Since min values are int32_t, shouldn't they be initialized with min.x = std::numeric_limits<int32_t>::max(); (instead of int64_t that it was before.
There was a problem hiding this comment.
To be honest this was trial and error.
I had tried what you suggested but then the whole result got scrambled. Like the higher the node level the more points were pulled towards the min xy of the complete dataset. Maybe because the viewer build upon this bug in decoding the Morton code.
Then I tried setting it 0 and the problem of coordinate differences got a little bit less.
The true solution was all the rounding that I introduced in stead of casting to int32_t.
There was a problem hiding this comment.
The original initialization appears to be doing this:
min = { -1, -1, -1 };
...as the result from int64_t max value conversions to int32_t.
There was a problem hiding this comment.
Perhaps this min struct could just be a constant, or totally eliminated. Point X, Y, Z values are relative to octree bounding box min, and always >= 0, are they not?
There was a problem hiding this comment.
Yes I think you're right. Just left it in there as Markus might want to throw in some other min values. But until now it was -1, -1, -1 indeed.
|
Any update on this? It might also fix some issues we've seen with LAS files being rejected because of wrongly classified points as being outside the bbox. |
|
I have the same issue with a LAZ file being rejected due to the bounding box check. I did verified the bounding box using The bounding box check code here did not compare I find this PR does fix my problem. Is there any update on when this will be merged? |
There is a problem in PotreeConverter that the coordinates (x, y and/or z) in the output differ in the last decimal from the input.
Consider the following example LAZ file (scale 0.001). It contains two points:
After conversion with PotreeConverter both points translate to X: 1.000, Y: 2.000, Z: 3.000.
This is due to the fact that Y of point 2 is read from LasZip as 2.0009999999.... Then with a cast to int32_t the last decimals are lost, and becomes 2.000.
decimal_error.zip
This pull request fixes that by applying some rounding.